-
Notifications
You must be signed in to change notification settings - Fork 34
Modernize Python and Django support: Drop Python 3.9, add Python 3.14 and Django 6.0 #489
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Update pyproject.toml: requires-python >= 3.10, add classifiers for Python 3.10-3.14 and Django 6.0 - Update tox.ini: remove py39, add py310, py312, py313, py314 - Update GitHub Actions workflows: add Python 3.14 and Django 6.0 test configurations - Add dj60_cms50.txt requirements file for Django 6.0 testing - Add exclusions to prevent Python 3.14 from running against older Django versions Amp-Thread-ID: https://ampcode.com/threads/T-cdf01cfa-6da3-4851-9a9b-0c7a2c181c37 Co-authored-by: Amp <[email protected]>
- Replace typing.Union[X, Y] with X | Y syntax (PEP 604) - Replace typing.Optional[X] with X | None syntax - Remove unnecessary typing imports where Optional/Union were the only imports - Updated files: admin.py, emails.py, conditions.py, helpers.py, cms_toolbars.py, indicators.py, datastructures.py Amp-Thread-ID: https://ampcode.com/threads/T-cdf01cfa-6da3-4851-9a9b-0c7a2c181c37 Co-authored-by: Amp <[email protected]>
- Removed unused typing imports from admin.py, conditions.py, emails.py, and indicators.py - These imports became unused after migrating to PEP 604 union syntax
- Updated Python version requirement from 3.9+ to 3.10+ - Added Django 6.0 to supported versions - Updated test matrix to reflect Python 3.10-3.14 - Added note about PEP 604 union syntax in type hints
Reviewer's GuideThis PR updates supported Python and Django versions by dropping Python 3.9, adding Python 3.14 and Django 6.0, modernizes type hints to PEP 604 syntax, and adjusts CI/CD and documentation to align with the new requirements. Class diagram for modernized type annotations in djangocms_versioningclassDiagram
class BaseVersionableItem {
+concrete: bool
+content_model: type[models.Model]
+content_admin_mixin: type | None
+__init__(content_model: type[models.Model], content_admin_mixin: type | None = None)
}
class VersionableItem {
+grouper_field_name: str
+copy_function: callable
+extra_grouping_fields: Iterable[str] | None
+version_list_filter_lookups: dict[str, Any] | None
+grouper_admin_mixin: type | None
+content_admin_mixin: type | None
+preview_url
+__init__(content_model: type[models.Model], grouper_field_name: str, copy_function: callable, extra_grouping_fields: Iterable[str] | None = None, version_list_filter_lookups: dict[str, Any] | None = None, grouper_admin_mixin: type | None = None, content_admin_mixin: type | None = None, preview_url = None)
+grouper_choices_queryset() models.QuerySet
+get_grouper_with_fallbacks(grouper_id) models.Model | None
+_get_content_types() set[int]
}
class VersionableItemAlias {
+to: BaseVersionableItem
+__init__(content_model: type[models.Model], to: BaseVersionableItem, content_admin_mixin: type | None = None)
}
BaseVersionableItem <|-- VersionableItem
BaseVersionableItem <|-- VersionableItemAlias
class VersioningPageToolbar {
+page_content: PageContent | None
+__init__(*args, **kwargs)
+get_page_content(language: str | None = None) PageContent
}
class Admin {
+get_versioning_state(obj: models.Model) str | None
+get_author(obj: models.Model) str | None
+get_modified_date(obj: models.Model) str | None
}
class Helpers {
+replace_admin_for_models(pairs: tuple[type[models.Model], type], admin_site: admin.AdminSite | None = None)
+register_versionadmin_proxy(versionable, admin_site: admin.AdminSite | None = None)
+get_preview_url(content_obj: models.Model, language: str | None = None) str
}
class Conditions {
+__add__(other: list) Conditions
+__get__(instance: object, cls) Conditions | BoundConditions
}
class Emails {
+get_full_url(location: str, site: Site | None = None) str
}
class Indicators {
+content_indicator(content_obj: models.Model, versions: list[Version] | None = None) str | None
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- The GitHub Actions matrix has grown quite verbose—consider switching to an include-based matrix or templated jobs to reduce duplication and avoid mismatched Python/Django combinations.
- After bumping requires-python to >=3.10, double-check that all dev tooling (e.g., tox envs, local setup docs) no longer reference Python 3.9 and still run correctly under the new minimum.
- AGENTS.md is a very large AI-specific guide in the repo root—consider moving it to a .github or docs/ directory to keep the root directory focused on code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The GitHub Actions matrix has grown quite verbose—consider switching to an include-based matrix or templated jobs to reduce duplication and avoid mismatched Python/Django combinations.
- After bumping requires-python to >=3.10, double-check that all dev tooling (e.g., tox envs, local setup docs) no longer reference Python 3.9 and still run correctly under the new minimum.
- AGENTS.md is a very large AI-specific guide in the repo root—consider moving it to a .github or docs/ directory to keep the root directory focused on code.
## Individual Comments
### Comment 1
<location> `djangocms_versioning/helpers.py:73` </location>
<code_context>
-def replace_admin_for_models(pairs: tuple[type[models.Model], type], admin_site: Optional[admin.AdminSite] = None):
+def replace_admin_for_models(pairs: tuple[type[models.Model], type], admin_site: admin.AdminSite | None = None):
"""
:param models: List of (model class, admin mixin class) tuples
</code_context>
<issue_to_address>
**issue:** Tuple type hint may be too restrictive for multiple pairs.
Consider updating the type hint to `Iterable[tuple[type[models.Model], type]]` if multiple pairs are expected, to better reflect the intended input.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #489 +/- ##
==========================================
+ Coverage 90.55% 93.64% +3.09%
==========================================
Files 72 76 +4
Lines 2732 2692 -40
Branches 322 0 -322
==========================================
+ Hits 2474 2521 +47
+ Misses 182 171 -11
+ Partials 76 0 -76 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Replace pip with uv for faster dependency installation - Add astral-sh/setup-uv@v5 action to all workflows - Use 'uv pip install --system' for package installation - Use 'uvx ruff' for linting workflow - Updated all test jobs: sqlite, postgres, mysql, cms-develop, django-main Amp-Thread-ID: https://ampcode.com/threads/T-cdf01cfa-6da3-4851-9a9b-0c7a2c181c37 Co-authored-by: Amp <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New security issues found
- Add note about using uv for faster dependency installation - Update testing command to use 'uv pip install' - Document uv as the package manager in Code Style section
- Change from 'tuple[type[models.Model], type]' to 'Iterable[tuple[type[models.Model], type]]' - The function expects multiple pairs, not a single tuple - Also fixed docstring parameter name from 'models' to 'pairs'
- Break function signature across multiple lines to comply with 120 char limit - Fixes ruff E501 error
Amp-Thread-ID: https://ampcode.com/threads/T-6e53f75e-8813-4ef9-bcd2-61db1b0e0cc6 Co-authored-by: Amp <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-7c01575b-a01e-4212-bfb3-f4981bd1583b Co-authored-by: Amp <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-1ae0efd4-afab-468c-b53b-30d0573c635e Co-authored-by: Amp <[email protected]>
|
@vinitkumar Can you take a look at #480 before we merge this? (Might contain some 3.9-specific |
…support' into feature/modernize-python-django-support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, @vinitkumar !
Shall we drop the version constraint for django-fsm-2, people might have other usages/requirements in their projects. So we can avoid version conflicts?
Finally, can we add a line to the readme explaining the move from django-fsm to django-fsm-2 due to its being actively maintained by Django Commons?
Looking forward to getting this released!
…support' into feature/modernize-python-django-support
|
@fsbraun Just did the changes you requested. Please have a look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice. Versioning made a good step forward.
This PR modernizes the project's Python and Django version support, following the approach from django-cms PR #8371.
Changes
Python Version Support
requires-pythonto >= 3.10Django Version Support
tests/requirements/dj60_cms50.txtfor Django 6.0 testingCode Modernization
typing.Union[X, Y]withX | Ytyping.Optional[X]withX | Nonetypingimports after modernizationCI/CD Improvements
Documentation
Testing Strategy
Checklist
Closes #XXX (if applicable)
Summary by Sourcery
Modernize the project’s Python and Django support by dropping Python 3.9, requiring Python >=3.10, adding Python 3.14 and Django 6.0 support, updating type hints to PEP 604 syntax, and refreshing CI, tox, and documentation accordingly.
New Features:
Enhancements:
Build:
Documentation:
Tests: